Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: provides access to the CSS variable created by CSSCustomProprtyBehaviors #3256

Merged

Conversation

nicholasrice
Copy link
Contributor

@nicholasrice nicholasrice commented Jun 9, 2020

Description

Adds a var field to CSSCustomPropertyBehaviors to provide the CSS Custom Property formatted as a CSS variable. This allows the string to be interpolated into stylesheets.

Also integrates the interpolation into stylesheets.

Question:
I waffled between CSSCustomPropertyBehavior.var and CSSCustomPropertyBehavior.css. I want to keep the property concise to benefit from minification processes but wasn't sure which felt more intuitive. If anyone prefers one or the other (or something else) please let me know, I'm not married to CSSCustomPropertyBehavior.var.

Motivation & context

Primary changes are in packages/web-components/fast-foundation/src/custom-properties/behavior.ts

Issue type checklist

  • Chore: A change that does not impact distributed packages.
  • Bug fix: A change that fixes an issue, link to the issue above.
  • New feature: A change that adds functionality.

Is this a breaking change?

  • This change causes current functionality to break.

Adding or modifying component(s) in @microsoft/fast-components checklist

Process & policy checklist

  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

@nicholasrice nicholasrice self-assigned this Jun 9, 2020
Copy link
Contributor

@EisenbergEffect EisenbergEffect left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out fo curiosity, did you run the build for the components before and after and notice a reduction in the minified size?

Also, I like var over css. That seems more descriptive.

@nicholasrice
Copy link
Contributor Author

@EisenbergEffect looks like a 3.244kb reduction in the fast-components.min.js file

@EisenbergEffect
Copy link
Contributor

@nicholasrice Nice!

@nicholasrice nicholasrice merged commit 391f029 into master Jun 10, 2020
@nicholasrice nicholasrice deleted the users/nirice/add-css-variable-access-to-css-behavior branch June 10, 2020 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants